New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(vite): unexptected overwriting for default export fix(#4553) #4596
Conversation
couldn't sleep :(. For example. A cjs package compile from esm format in node_modules // node_modules/foo/index.js
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.a = void 0;
var a = function helloA() { };
exports.a = a;
var foo = function hello() { };
exports.default = foo; the package exports a object like // cjsFoo
const foo = {
helloA: function helloA() {},
default: function hello() {},
__esModule: { value: true },
} vite will pre-bundle this package to an esm package. simply It will bundle into // esmFoo
const foo = {
helloA: function helloA() {},
default: function hello() {},
}
export { foo as default } So, when we write something like The problem is shown in dynamic import. Let's continue the case. m = {
default: { // cjsFoo
helloA: function helloA() {},
default: function hello() {},
__esModule: { value: true },
}
} and after dynamic-foo = {
default: { // cjsFoo
helloA: function helloA() {},
default: function hello() {},
__esModule: { value: true },
},
helloA: function helloA() {},
__esModule: { value: true },
} Thus, It causes the problem in #4553. dynamic-foo = {
helloA: function helloA() {},
default: function hello() {},
__esModule: { value: true },
} |
Could you add some tests for it? Thanks |
@@ -15,6 +15,8 @@ | |||
"dep-linked": "link:./dep-linked", | |||
"dep-linked-include": "link:./dep-linked-include", | |||
"dep-esbuild-plugin-transform": "link:./dep-esbuild-plugin-transform", | |||
"dep-cjs-compiled-from-esm": "file:./dep-cjs-compiled-from-esm", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It really confusing me that if I use link:
instead of file:
Vite won't pre-bundle my package. Not sure whether it is a bug too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe vite gets a real address with no node_modules
in the path, so it's not pre-bundle
vite/packages/vite/src/node/optimizer/scan.ts
Lines 298 to 302 in eef51cb
if (resolved.includes('node_modules') || include?.includes(id)) { | |
// dependency or forced included, externalize and stop crawling | |
if (OPTIMIZABLE_ENTRY_RE.test(resolved)) { | |
depImports[id] = resolved | |
} |
this issue comment analyzes the issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It looks like file:
will make npm/yarn copy the package to node_modules rather than link it.
@antfu done. |
vitejs#4596) * fix(vite): unexptected overwriting for default export fix(vitejs#4553) * feat: add test * feat: delete unused package
Description
fix #4553.
I just found the cause, allow me to take a sleep then add some tests
and description:).Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).